Implement new lint iter_over_hash_type
#11791
Conversation
|
r? @Jarcho (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
Jacherr
force-pushed
the
iter_over_hash_type
branch
from
084792b to
7bc39f3
Compare
| && (match_def_path(cx, did, &HASHMAP_KEYS) | ||
| || match_def_path(cx, did, &HASHMAP_VALUES) | ||
| || match_def_path(cx, did, &HASHSET_ITER_TY) | ||
| || is_type_diagnostic_item(cx, ty, sym::HashMap) | ||
| || is_type_diagnostic_item(cx, ty, sym::HashSet)) |
There was a problem hiding this comment.
This is missing HashSet::IterMut, HashMap::Iter, HashMap::IterMut and HashMap::ValuesMut. Drain for both collections should probably also be caught.
There was a problem hiding this comment.
HashSet::IterMut doesn't appear to exist, but I'll add the rest.
| /// | ||
| /// ### Why is this bad? | ||
| /// Because hash types are unordered, when iterated through such as in a for loop, the values are returned in | ||
| /// a pseudo-random order. As a result, on redundant systems this may cause inconsistencies and anomalies. |
There was a problem hiding this comment.
nit: an undefined order
'pseudo-random' is not technically wrong, but very misleading.
| cx, | ||
| ITER_OVER_HASH_TYPE, | ||
| expr.span, | ||
| "iterating over unordered hash-based type", |
rustbot
added
S-waiting-on-author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
I'd like tests with type aliases as well, like FxHashMap. This should work since you use the typeck table but it should be linted
Jacherr
force-pushed
the
iter_over_hash_type
branch
from
0d686fa to
f8ea496
Compare
|
Thank you. @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
5 participants
Implements and fixes #11788
This PR adds a new restriction lint
iter_over_hash_typewhich preventsHash-types (that is,HashSetandHashMap) from being used as the iterator inforloops.The justification for this is because in
Hash-based types, the ordering of items is not guaranteed and may vary between executions of the same program on the same hardware. In addition, it reduces readability due to the unclear iteration order.The implementation of this lint also ensures the following:
HashMap::keys,HashMap::values, andHashSet::iterare also denied when used inforloops,changelog: add new
iter_over_hash_typelint to prevent unordered iterations through hashed data structures